Skip to content

Avoid per-row queries for evolution condition variables and natures - #1690

Open
santichausis wants to merge 2 commits into
PokeAPI:masterfrom
santichausis:perf/evolution-detail-lookups
Open

santichausis wants to merge 2 commits into
PokeAPI:masterfrom
santichausis:perf/evolution-detail-lookups

Conversation

@santichausis

@santichausis santichausis commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Problem

PokemonEvolutionSerializer.get_condition_expression and get_allowed_natures run one query per PokemonEvolution row. Milcery → Alcremie has 63 evolution rows, each with a condition_expression, so /evolution-chain/452/ takes 65 queries (63 of them to pokemon_v2_evolutionvariable).

Fix

Both tables are tiny (4 evolution variables, 25 natures). Each is loaded and serialized once, lazily, through a cached_property on the serializer instance, and each row picks its entries in Python, keeping the previous ordering. Variables are kept as (symbol, data) pairs rather than a dict, since symbol isn't unique across version groups. ListSerializer reuses a single child instance for every row, so this is one query per list of evolution details instead of one per row, and no state is shared across requests.

Verification

  • Built the DB from the CSVs and serialized all 540 evolution chains before/after: byte-identical JSON for every chain, chain 452 drops from 65 to 3 queries (and its serialization from ~52 ms to ~32 ms locally, by not re-running the summary serializers per row), and no chain needs more. OpenAPI schema unchanged.
  • Added a test that builds chains with 1 and 5 evolution rows (each with a condition_expression and nature_bitmask), asserts the query count is the same, and checks the resolved variables and natures. This is the first API test coverage for these two fields. It uses cachalot_disabled(), because inside the test transaction cachalot serves identical repeated queries from memory, which hides per-row queries. On master this test fails with 13 vs 5 queries.
  • manage.py test pokemon_v2 (64 tests), ruff, and ty (no new diagnostics) pass.
  • In production cachalot/Redis caches these queries, so the gain is mainly for cold-cache requests and fewer cache round-trips.
  • Touches the same file as Fix N+1 query in evolution chain serializer #1688; whichever lands second will need a trivial rebase.

Comment thread pokemon_v2/serializers.py Outdated
get_condition_expression and get_allowed_natures ran a query per
PokemonEvolution row. Milcery -> Alcremie has 63 rows, each with a
condition_expression, so its evolution chain took 65 queries.

Both tables are tiny (4 evolution variables, 25 natures), so load each
once, lazily, on the serializer instance. A ListSerializer reuses its
child instance for every row, so this is one query per list instead of
one per row, without sharing state across requests.
@santichausis
santichausis force-pushed the perf/evolution-detail-lookups branch from 003f0ff to 7f96c55 Compare September 30, 2026 11:53
The cached lookups still ran the summary serializers once per
evolution row. Serialize each table once alongside the query and reuse
the results; serializing Alcremie's chain (63 rows) drops from ~52 ms
to ~32 ms locally with identical output.

Variables are kept as (symbol, data) pairs instead of a dict, since
symbol isn't unique across version groups.
@FallenDeity

Copy link
Copy Markdown
Contributor

lgtm :) thanks, just remove the comments on top of the properties

    # a list of pairs rather than a dict, since symbols aren't unique across version groups

its evident in the code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants